CB-7684: (iOS) Fix CDVSound killing all audio when a single file finishes #100

tbrebant · 2016-06-17T08:14:03Z

Platforms affected

iOS

What does this PR do?

We arrived to the same conclusion as Nathan Stryker on Jira (https://issues.apache.org/jira/browse/CB-7684) and @ionut-movila in the PR #33 : all self.avSession setActive:NO should be removed.

The problem was not only that when a media finishes it is stopping other ones played by the plugin, but it is also killing any sound played by another plugin or by WebView's default webaudio system, triggering a descriptive:

AVAudioSession.mm:646: -[AVAudioSession setActive:withOptions:error:]: Deactivating an audio session that has running I/O. All I/O should be stopped or paused prior to deactivating the audio session.

We found that AVAudioSession is a singleton for the whole app (cf. https://developer.apple.com/library/ios/documentation/AVFoundation/Reference/AVAudioSession_ClassReference/).

Apple says:

Most apps never need to deactivate their audio session explicitly. Important exceptions include VoIP (Voice over Internet Protocol) apps, turn-by-turn navigation apps, and, in some cases, recording apps.

(cf. https://developer.apple.com/library/ios/documentation/Audio/Conceptual/AudioSessionProgrammingGuide/ConfiguringanAudioSession/ConfiguringanAudioSession.html)

What testing has been done on this change?

Played multiple sounds (non streaming) with the plugin, then waited for one to stop: it is not stopping other sounds anymore.
Played multiple sounds (non streaming) with the plugin, then stopped one and released it: it is not stopping other sounds anymore.
Played a sound (non streaming) with the plugin and another one via webview's webaudio, then waited the one played by the plugin to end: it is not stopping other sounds anymore.
Played a sound (non streaming) with the plugin and another one via webview's webaudio, then stopped and released the one played by the plugin: it is not stopping other sounds anymore.
Checked all possibilities with Xcode monitoring application: no memory issue detected.

Ran the integrated tests. Before doing any change (version 2.3.1-dev) the result was:

After the change the result is exactly the same:

All tests was made with an iPad2 on iOS8.4.

Before merging, it would be great if someone can test:

an application using the recording feature
an application using the streaming feature

Checklist

ICLA has been signed and submitted to [email protected].
Reported an issue in the JIRA database
Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
Added automated test coverage as appropriate for this change.

About the last point: I don't see what kind of tests we can add for this fix.

…shes

cordova-qa · 2016-06-17T08:28:34Z

Cordova CI Build has one or more failures.

Commit - Link
Dashboard - Link

Builder Name	Console Output	Test Report	Device Logs
Windows 8.1 Store	Link	Link	Link
Windows 10 Store	Link	Link	Link
Windows 8.1 Phone	Link	Link	Link
iOS	Link	Link	Link
Android	Link	Link	Link

tbrebant · 2016-08-25T01:30:57Z

What about this? Would it be possible to have a comment or an opinion?

kaynz · 2016-08-30T09:17:23Z

@cordova-qa +1 on this.

katzlbt · 2016-11-17T10:33:16Z

It looks like this fix includes the pull request:
"Fix app halt when an audio ends w/ another playing" #105

katzlbt

I have integared this patch into version 2.4.0 and tested playback with iOS10. It works.

anagrath · 2017-04-13T06:42:52Z

This issue is still on version 3.0.0 -- I am not sure if all setActive:NO should be commented out -- it seems that there is some logic to kill the audio playback when recording? I'd much prefer a stable version update than to do my own hacking... what can I do to help?

katzlbt · 2017-04-13T08:56:32Z

Sending setActiveNo: to an audio session will always stop recording and playback. This is how the system handles incoming phonecalls etc.

Most apps never need to deactivate their audio session explicitly. Important exceptions include VoIP (Voice over Internet Protocol) apps, turn-by-turn navigation apps, and, in some cases, recording apps. (https://developer.apple.com/library/content/documentation/Audio/Conceptual/AudioSessionProgrammingGuide/ConfiguringanAudioSession/ConfiguringanAudioSession.html)

Probably you are out of luck for waiting it out, unless you have a year or so. This PR and another that fixes that Audio is disabled after every memory warning are open for ages (#120). I apply the patches manually like this:

            wget https://github.com/apache/cordova-plugin-media/pull/120.patch
            patch -p1 <120.patch
            
            wget https://github.com/apache/cordova-plugin-media/pull/116.patch
            patch -p1 <116.patch
            
            wget https://github.com/apache/cordova-plugin-media/pull/100.patch
            patch -p1 <100.patch

adriano-di-giovanni · 2017-06-22T09:43:27Z

I've just submitted a #143 PR that solves the issue implementing an CDVSound#isPlayingOrRecording method. The method checks the status of player/recorder for every CDVAudioFile in soundCache. The method is used in conjunction with the avSession != null check wherever the plugin wants to deactivate avSession.

iamrototo · 2017-10-09T07:51:52Z

Hello, I have a similar problem for ages : after I record an audio, I can't play any other audio files (using default HTML5 audio feature).

I have succeed to fix this issue by doing a bit more work than you did.
You can see all the changes here:

iamrototo@ca374fd

damienleroux · 2017-12-20T16:00:07Z

+1. have the changes been merged?

I test it and it worked for me. I forked this repo and publish (cordova add [email protected]) my own plugin based on your fix.

Thanks All

macdonst · 2018-01-02T16:34:07Z

@damienleroux This PR has a merge conflict. It would probably be better for you to send a new PR with the required changes. You can reference this one so we can close it as well.

kaynz · 2018-01-03T06:54:48Z

@macdonst Is there any realistic chance that the new PR will be merged contemporary and not ignored for another year?

damienleroux · 2018-01-03T07:26:28Z

Done: #162.

Thank you :)

adriano-di-giovanni · 2018-01-03T09:24:11Z

@damienleroux, @macdonst are you talking about my #143 being merged? If so, I've just updated my PR to fix merge conflicts that were related to plugin.xml only. Please, let me know.

damienleroux · 2018-01-03T13:10:57Z

@adriano-di-giovanni I didn't see you submit a PR for this issue. May be more relevant that the commit from@tbrebrant that solves the issue: I don't know.

adriano-di-giovanni · 2018-01-03T16:24:17Z

@damienleroux It's the #143. Did I make any mistakes? I think it's a valid alternative to the one from @tbrebant. It's also backward compatible. Please, let me know.

damienleroux · 2018-01-03T18:20:32Z

@adriano-di-giovanni. I can't say if you made mistakes or not. Besides, @macdonst, my PR #162 seems to fail travis integration tests but don't know why.

kaynz · 2018-01-04T06:14:09Z

@damienleroux Pretty sure it's an environment bug and has nothing to do with your commit: https://travis-ci.org/apache/cordova-plugin-media/jobs/324476963#L1534

macdonst · 2018-01-06T00:15:57Z

Closing in favour of #143 or #162 which merge cleanly.

CB-7684: (iOS) Fix CDVSound killing all audio when a single file fini…

40a561d

…shes

katzlbt approved these changes Nov 17, 2016

View reviewed changes

macdonst closed this Jan 6, 2018

CB-7684: (iOS) Fix CDVSound killing all audio when a single file finishes #100

CB-7684: (iOS) Fix CDVSound killing all audio when a single file finishes #100

Uh oh!

Conversation

tbrebant commented Jun 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Platforms affected

What does this PR do?

What testing has been done on this change?

Checklist

Uh oh!

cordova-qa commented Jun 17, 2016

Uh oh!

tbrebant commented Aug 25, 2016

Uh oh!

kaynz commented Aug 30, 2016

Uh oh!

katzlbt commented Nov 17, 2016

Uh oh!

katzlbt left a comment

Choose a reason for hiding this comment

Uh oh!

anagrath commented Apr 13, 2017

Uh oh!

katzlbt commented Apr 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriano-di-giovanni commented Jun 22, 2017

Uh oh!

iamrototo commented Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damienleroux commented Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

macdonst commented Jan 2, 2018

Uh oh!

kaynz commented Jan 3, 2018

Uh oh!

damienleroux commented Jan 3, 2018

Uh oh!

adriano-di-giovanni commented Jan 3, 2018

Uh oh!

damienleroux commented Jan 3, 2018

Uh oh!

adriano-di-giovanni commented Jan 3, 2018

Uh oh!

damienleroux commented Jan 3, 2018

Uh oh!

kaynz commented Jan 4, 2018

Uh oh!

macdonst commented Jan 6, 2018

Uh oh!

Uh oh!

tbrebant commented Jun 17, 2016 •

edited

Loading

katzlbt commented Apr 13, 2017 •

edited

Loading

iamrototo commented Oct 9, 2017 •

edited

Loading

damienleroux commented Dec 20, 2017 •

edited

Loading